Conversation
…ad of change to int16 (maybe)
MSVC #define private public incompatibility (tests/db/index/CMakeLists.txt): Excluded 4 tests on MSVC (mmap_store_test, mem_store_test, inverted_indexer_util_test, segment_test) because MSVC encodes access specifiers in mangled names, making the #define private public hack fundamentally incompatible.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
CMakeLists.txt
Outdated
| add_compile_options(/wd4245 /wd4334 /wd4702 /wd4305) | ||
|
|
||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror=return-type") | ||
| set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror=return-type") | ||
| # //warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) | ||
| # keeps |= (1 << k); | ||
| add_compile_options(/wd4334) | ||
| ###### | ||
|
|
There was a problem hiding this comment.
/wd4334 suppressed twice
Warning C4334 ("result of 32-bit shift implicitly converted to 64 bits") is already listed in the combined add_compile_options(/wd4245 /wd4334 /wd4702 /wd4305) line above, and then suppressed a second time individually. The duplicate is harmless but indicates a copy-paste oversight.
| add_compile_options(/wd4245 /wd4334 /wd4702 /wd4305) | |
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror=return-type") | |
| set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror=return-type") | |
| # //warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) | |
| # keeps |= (1 << k); | |
| add_compile_options(/wd4334) | |
| ###### | |
| add_compile_options(/wd4245 /wd4702 /wd4305) | |
| # warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) | |
| # keeps |= (1 << k); | |
| add_compile_options(/wd4334) |
| add_compile_options(/wd4146) # unary minus operator applied to unsigned type #usage: uint32_t seg_id_{-1U}; | ||
| add_compile_options(/wd4310) # warning C4310: cast truncates constant value |
There was a problem hiding this comment.
Broad narrowing-conversion warnings suppressed without fix
/wd4267 (conversion from size_t to a smaller type) and /wd4244 (general type narrowing) are silently disabled with a comment acknowledging they need to be resolved. These two warnings regularly surface real runtime bugs (e.g., size_t → int truncation on 64-bit data, unintended float→int narrowing). Leaving them suppressed while the PR is merged makes it easy for future changes to introduce silent truncation bugs that are never caught.
Consider replacing the blanket suppression with targeted #pragma warning(suppress: ...) at the specific call sites that are genuinely safe, rather than disabling them project-wide.
| namespace core { | ||
|
|
There was a problem hiding this comment.
key_t type alias defined in two headers
using key_t = uint64_t is introduced here inside namespace zvec::core, and an identical alias is also added to src/core/utility/sparse_utility.h (line 27) in the same namespace. Any translation unit that includes both headers will have a duplicate declaration. While C++ allows re-declaring a type alias to the same type, this duplication is fragile — a future change to one without the other creates a subtle mismatch. The alias should live in a single shared header (e.g. a types.h in src/core) and be included by both files.
JalinWang
force-pushed
the
feat/win-compilation
branch
from
ffc9d32 to
5c61212
Compare
# Conflicts: # cmake/bazel.cmake # src/ailego/math/inner_product_matrix_fp16.cc # src/ailego/math/inner_product_matrix_fp32.cc # src/ailego/math/mips_euclidean_distance_matrix_fp32.cc
|
Update: just got it building on Win Server 2025 / VS2022 and the UTs are passing.
|
# Conflicts: # tests/core/algorithm/flat/flat_streamer_buffer_test.cc # tests/core/algorithm/flat/flat_streamer_buffer_time_test.cpp
# Conflicts: # src/ailego/math/inner_product_matrix.h # src/ailego/math/inner_product_matrix_fp16_avx512.cc # tests/core/interface/index_interface_test.cc
| ###### | ||
|
|
||
| #TODO(windows): to be verified | ||
| add_definitions(-DARROW_STATIC -DPARQUET_STATIC -DARROW_ACERO_STATIC -DARROW_DS_STATIC -DARROW_COMPUTE_STATIC) |
There was a problem hiding this comment.
这个有点过于hack了吧。。不加难道会有问题吗？
There was a problem hiding this comment.
As the TODO comment suggests, this needs re-verification and refinement. I'll address it after the Windows CI is set up.
|
|
||
|
|
||
| ###### reduce output length to make vibe coding work better :), which should be removed or solved later | ||
| add_compile_options(/wd4267 /wd4244) |
There was a problem hiding this comment.
这部分warning的编号可以统一放在一个add_compile_options里头吧，并且加上每个编号的注释。
|
|
||
| include_directories(${PROJECT_ROOT_DIR}/src/include) | ||
| include_directories(${PROJECT_ROOT_DIR}/src) | ||
| include_directories(${PROJECT_ROOT_DIR}) |
There was a problem hiding this comment.
To include tests/test_util.h, which contains definitions for sleep and a wildcard-supporting remove function. I'll clean this up later by moving sleep to platform.h and removing wildcard usage from the UTs.
| IMPORTED_LOCATION_RELEASE "${lz4_LIBRARY}" | ||
| IMPORTED_LOCATION_MINSIZEREL "${lz4_LIBRARY}" | ||
| IMPORTED_LOCATION_RELWITHDEBINFO "${lz4_LIBRARY}" | ||
| INTERFACE_INCLUDE_DIRECTORIES "${lz4_INCLUDE_DIR}" |
There was a problem hiding this comment.
这里是不是加个if(win)的判断会清晰一些
thirdparty/arrow/CMakeLists.txt
Outdated
| # $<CONFIG> is a generator expression that evaluates to the current build configuration (e.g., Debug, Release) at build system generation time, not at CMake configure time. | ||
| # Multi-config vs. single-config generators: | ||
| # Multi-config generators (Visual Studio, Xcode, Ninja Multi-Config): $<CONFIG> works as expected and evaluates differently for each configuration. | ||
| # Single-config generators (Makefiles, Ninja): $<CONFIG> evaluates to the value of CMAKE_BUILD_TYPE (which must be set at configure time) |
There was a problem hiding this comment.
It was meant to explain the changes about BUILD_COMMAND "${CMAKE_COMMAND}" --build . -j ${NPROC} --config $<CONFIG>; removed now
2 participants
WIP